fix(drivers): expose device limits and measured control response - #149
Conversation
Signed-off-by: Fredrik Ahlgren <fredrik@sourceful-labs.com>
Codex Review SummaryThis comment shows the latest Codex review activity on this pull request.
ℹ️ About Codex in GitHubYour team has set up Codex to review pull requests in this repo. Reviews are triggered when you
Codex reacts with 👀 while any review is running, comments if it has suggestions, and reacts with 👍 once all reviews finish with no findings. |
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 405ca9a927
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| local battery = { | ||
| w = bat_w, | ||
| setpoint_w = setpoint_pixii_w and -setpoint_pixii_w, | ||
| control_power_w = acw_regs and ac_w, |
There was a problem hiding this comment.
When the battery discharges, register 40083 is the SunSpec inverter AC-power value, whose positive direction is inverter output; other Model 103 consumers such as drivers/lua/solaredge_pv.lua:203-243 negate this register before emitting the repository's site sign. Passing ac_w through here therefore reverses command feedback in both charge and discharge, so Core will compare a site-signed setpoint with an oppositely signed response. Emit -ac_w and test both power directions.
AGENTS.md reference: AGENTS.md:L71-L73
Useful? React with 👍 / 👎.
| control_power_w = acw_regs and ac_w, | ||
| control_power_available = acw_regs ~= nil, |
There was a problem hiding this comment.
If the separate read of register 40084 fails while 40083 succeeds, read_sf silently substitutes scale factor zero, yet this code reports the resulting raw register value as available control power. A transient failure can therefore make Core treat a value that is off by the device's power multiplier as fresh command feedback; availability should require successful reads of both the value and its scale factor, preferably from one block read.
AGENTS.md reference: AGENTS.md:L17-L20
Useful? React with 👍 / 👎.
Signed-off-by: Fredrik Ahlgren <fredrik@sourceful-labs.com>
Signed-off-by: Fredrik Ahlgren <fredrik@sourceful-labs.com>
Paired with srcfl/ftw#1474, which now reads control_power_confirmed. Easee's cloud records power only on change, so a steady charge kept an old source time and Core raised a lost-control alarm during normal charging. The driver now marks the unchanged value confirmed while the cloud still hears from the charger; the source time stays for distinct samples. Pixii's AC power (40083) is SunSpec generator frame, like its setpoint, so control_power_w is now negated into site signs. A following battery read as the wrong direction before. Hardware has not verified either sign yet. Sungrow claims external_meter only when the meter reads power or phase current; a meterless install reads zero and claims none. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> Signed-off-by: Fredrik Ahlgren <fredrik@sourceful-labs.com>
Moves the bundled snapshot to srcfl/device-drivers#149 at d74ace6a: Easee marks a steady value confirmed while the cloud hears from the charger, Pixii's control power leaves SunSpec's generator frame, and Sungrow claims a separate meter only when one reads power or phase current. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> Signed-off-by: Fredrik Ahlgren <fredrik@sourceful-labs.com>
Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> Signed-off-by: Fredrik Ahlgren <fredrik@sourceful-labs.com>
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: df003935ef
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
|
|
||
| host.emit("pv", { | ||
| w = -pv_w, -- negative = generation (EMS convention) | ||
| control_power_available = pv_regs ~= nil or mppt_regs ~= nil, |
There was a problem hiding this comment.
When the primary PV register returns zero because of the documented firmware quirk and the MPPT read times out, this or still marks control power available even though the fallback ladder emits zero without a usable source. Core can consequently accept a false zero-power response while the array is still generating; availability should require whichever source actually determined pv_w to have answered.
AGENTS.md reference: AGENTS.md:L18-L20
Useful? React with 👍 / 👎.
| -- power and zero phase current, so it claims no separate meter. | ||
| local meter_origin = nil | ||
| local meter_reads = mw_regs ~= nil and (meter_w ~= 0 or l1_a + l2_a + l3_a > 0) | ||
| if model_family == "hybrid" and meter_reads then meter_origin = "external_meter" end |
There was a problem hiding this comment.
This condition covers every device classified as a hybrid, including the advertised SH-RS and SH-T families, but the only upstream evidence added in manifests/sungrow.yaml:56-59 is the SH-RT manual. On the unevidenced families, any nonzero 5600/current reading is therefore promoted to an independent external meter that Core may use to confirm control, without proof that those registers represent a separate sensor rather than inverter-derived data; restrict the claim to evidenced models or add model-specific register/HIL evidence.
AGENTS.md reference: AGENTS.md:L17-L20
Useful? React with 👍 / 👎.
* feat(control): show command limits and measured response tiers Signed-off-by: Fredrik Ahlgren <fredrik@sourceful-labs.com> * fix(control): require fresh source samples for measured confirmation Signed-off-by: Fredrik Ahlgren <fredrik@sourceful-labs.com> * feat(control): compare site response curves with measured flow changes Signed-off-by: Fredrik Ahlgren <fredrik@sourceful-labs.com> * fix(control): classify commanded devices and qualify solar response Signed-off-by: Fredrik Ahlgren <fredrik@sourceful-labs.com> * Recognize relayed physical meters and show measured time gaps Signed-off-by: Fredrik Ahlgren <fredrik@sourceful-labs.com> * Verify control per device with unknown flows in the residual Signed-off-by: Fredrik Ahlgren <fredrik@sourceful-labs.com> * Put control proof on device bubbles and open evidence on tap Signed-off-by: Fredrik Ahlgren <fredrik@sourceful-labs.com> * Keep control evidence quiet on overview bubbles Signed-off-by: Fredrik Ahlgren <fredrik@sourceful-labs.com> * Open device controls first with measured evidence below Signed-off-by: Fredrik Ahlgren <fredrik@sourceful-labs.com> * Keep control evidence below the device controls in the vision Signed-off-by: Fredrik Ahlgren <fredrik@sourceful-labs.com> * Separate measured power evidence from target fulfilment Signed-off-by: Fredrik Ahlgren <fredrik@sourceful-labs.com> * Stop charging full batteries and explain the pause Signed-off-by: Fredrik Ahlgren <fredrik@sourceful-labs.com> * Answer "Are we in control?" in every mode and name the evidence Control proof restarted on every command that differed by 100 W, so modes that retune the battery each tick showed "waiting" most of the time and never warned when the battery stopped following. A cloud charger never reached measured proof, a kettle after a confirmed step removed the confirmation, and a battery tapering near full raised a warning. Retuning now continues one measurement record. Each reading is compared with the commands that could still be in force during the device's response time; only a material step (500 W or the tolerance) starts a new comparison. A source's declared power_max_age_s sets its freshness everywhere. The site comparison freezes once its post-step window is complete. A charge shortfall at 90% SoC or more, or a discharge shortfall at 10% or less, is information. Core now states the answer: status (following, waiting, limited, not_following, no_contact, not_controlled), severity (info, warning, alarm), evidence (accepted, measured, confirmed), confirmed_at_ms and readings_fresh. Lost measured proof becomes readings_lost. Site numbers live in site_evidence and are omitted when not computed. Trace tests replay self-consumption, an ignored battery, a kettle, a taper and a cloud charger. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> Signed-off-by: Fredrik Ahlgren <fredrik@sourceful-labs.com> * Show the control answer first in each device sheet A tap on a bubble opened the manual hold form with "Charge 3000 W" preselected, and the answer sat folded below it in tier labels. The sheet now starts with "Are we in control?": one status, one sentence with the measured numbers and, when the owner can act, the next step. "How FTW knows" lists sent, accepted, measured and confirmed evidence, with numbers and the response curve folded inside. Manual override folds below; Stop sits in the active-hold banner. The overview stays quiet while FTW is in control. Only Core's warning and alarm draw a mark, an amber triangle or a red disc, and a combined bubble keeps the worst. The Values view lists each controlled device with its answer. Words come from Core's status, severity and reason; a test fails when Core can emit a reason without words. The EV status line keeps power first. The shared module holds no DOM code; the box renderer lives in control-feedback-view.js. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> Signed-off-by: Fredrik Ahlgren <fredrik@sourceful-labs.com> * Describe control status and the evidence receipt Replace the tier ladder and bubble layout rules in VISION.md with the status the owner sees and the receipt behind it. Keep the paragraph about battery learning, but after the roadmap table so the Later row stays in it. The changeset now describes the shipped behaviour. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> Signed-off-by: Fredrik Ahlgren <fredrik@sourceful-labs.com> * Describe measured changes in words instead of signs The app never shows a bare minus sign. Device, grid and unexplained changes now read "toward discharge", "toward export" or "more drawn". Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> Signed-off-by: Fredrik Ahlgren <fredrik@sourceful-labs.com> * Keep change-only sources current while they confirm the value Easee's cloud records power only when it changes; a steady charge kept an old source time, so measured proof lapsed and the lost-control alarm fired during normal charging. A driver may now set control_power_confirmed when the source still hears from the device and the value is unchanged. Core treats that reading as a measurement at the time it arrived: it keeps a steady charge measured, and a charger that holds its old power after a new target now reads as not following instead of waiting forever. Sources that do not confirm keep the old rule: a repeated sample adds nothing. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> Signed-off-by: Fredrik Ahlgren <fredrik@sourceful-labs.com> * Pin drivers with confirmed Easee power and site-signed Pixii AC Moves the bundled snapshot to srcfl/device-drivers#149 at d74ace6a: Easee marks a steady value confirmed while the cloud hears from the charger, Pixii's control power leaves SunSpec's generator frame, and Sungrow claims a separate meter only when one reads power or phase current. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> Signed-off-by: Fredrik Ahlgren <fredrik@sourceful-labs.com> --------- Signed-off-by: Fredrik Ahlgren <fredrik@sourceful-labs.com> Co-authored-by: Claude Opus 5.5 <noreply@anthropic.com>
1 participant
An Easee charger can cap charging at 8 A while FTW asks for 16 A. Pixii can accept a write and report a different setpoint on a later poll. These changes expose those facts during normal operation, so Core can tell whether a device does what FTW asked.
Pixii 2.1.7 reads the setpoint outside troubleshooting mode and reports AC power for control separately from its DC battery reading. AC power (40083) is SunSpec's generator frame, like the setpoint, so it is negated into site signs; a following battery read as the wrong direction before. Easee Cloud 1.3.6 reports its static current ceiling and age separately from the dynamic offer. It keeps the source timestamp when the cloud repeats an observation and marks the unchanged value
control_power_confirmedwhile the cloud still hears from the charger, so a steady charge stays measured instead of raising a lost-control alarm. Missing power or source time cannot confirm a command's effect.Sungrow 1.5.10 reports the active forced setpoint from its existing holding-register read. It claims a separate meter (
power_origin="external_meter") on known hybrid models only when the meter reads power or phase current; a meterless install reads zero and claims none. Failed meter or PV reads cannot supply control evidence. No extra Modbus calls.A failed settings read does not refresh the limit's age. Existing polling cadence and write policy stay unchanged.
Paired with srcfl/ftw#1474 (pins d74ace6) and srcfl/ftw-webapp#75.
Validation:
make check(4,754 passed, 913 skipped). Focused: sungrow 88 passed, easee_cloud 38 passed, pixii 104 passed, including a meterless hybrid, a confirmed steady Easee value and Pixii's AC sign. A private ARM64 box run confirmed the Sungrow setpoint readback and fresh measured power. Pixii hardware checks remain, for both signs; keep draft. Known gaps: SH-RS and SH-T share the hybrid claim without manual evidence, and Sungrow's 5 kW write clamp reads as a changed setpoint when Core sends more. Existing Heishamon PRs #60 and #66 remain untouched.🤖 Generated with Claude Code